feat(providers): add Dockerfile/Containerfile provider for image analysis#569
Conversation
…ysis Add a new provider that recognizes Dockerfile and Containerfile manifests for component and stack analysis. The provider parses FROM lines to extract the base image reference, then reuses generateImageSBOM to produce a CycloneDX SBOM. Multi-stage Dockerfiles use the final stage's FROM image. Implements TC-4937 Assisted-by: Claude Code
Reviewer's GuideAdds a new OCI Dockerfile/Containerfile provider that parses FROM lines to derive the base image, generates an image SBOM via existing OCI utilities, wires it into the provider registry, and introduces unit tests around support detection, parsing, and behavior. Sequence diagram for Dockerfile provider image SBOM generationsequenceDiagram
actor Client
participant dockerfileProvider
participant fs
participant parseFromImage
participant parseImageRef
participant generateImageSBOM
Client->>dockerfileProvider: provideComponent(manifest, opts)
dockerfileProvider->>dockerfileProvider: getImageSBOM(manifest, opts)
dockerfileProvider->>fs: readFileSync(manifest, utf-8)
fs-->>dockerfileProvider: manifestContent
dockerfileProvider->>parseFromImage: parseFromImage(manifestContent)
parseFromImage-->>dockerfileProvider: image
dockerfileProvider->>parseImageRef: parseImageRef(image, opts)
parseImageRef-->>dockerfileProvider: imageRef
dockerfileProvider->>generateImageSBOM: generateImageSBOM(imageRef, opts)
generateImageSBOM-->>dockerfileProvider: sbom
dockerfileProvider-->>Client: {ecosystem: oci, content, contentType}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
parseFromImageimplementation assumes a relatively simpleFROMsyntax and only strips a single leading--flag; consider making this parsing more robust (e.g., handling multiple flags, comments, and more complex whitespace) or delegating to a Dockerfile parser to avoid mis-extracting the base image. - In
getImageSBOM, synchronousfs.readFileSyncis used; if other providers are asynchronous or this runs in a performance-sensitive path, consider aligning with the existing I/O pattern to avoid blocking the event loop on large Dockerfiles.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `parseFromImage` implementation assumes a relatively simple `FROM` syntax and only strips a single leading `--flag`; consider making this parsing more robust (e.g., handling multiple flags, comments, and more complex whitespace) or delegating to a Dockerfile parser to avoid mis-extracting the base image.
- In `getImageSBOM`, synchronous `fs.readFileSync` is used; if other providers are asynchronous or this runs in a performance-sensitive path, consider aligning with the existing I/O pattern to avoid blocking the event loop on large Dockerfiles.
## Individual Comments
### Comment 1
<location path="src/providers/oci_dockerfile.js" line_range="39-44" />
<code_context>
+ * @returns {string} the image reference from the last FROM line
+ * @throws {Error} when no FROM line is found in the Dockerfile
+ */
+export function parseFromImage(manifestContent) {
+ const lines = manifestContent.split(/\r?\n/)
+ let lastFrom = null
+ for (const line of lines) {
+ const trimmed = line.trim()
+ if (/^FROM\s+/i.test(trimmed)) {
+ // Extract image ref: FROM [--platform=...] image [AS name]
+ const withoutFrom = trimmed.replace(/^FROM\s+/i, '')
</code_context>
<issue_to_address>
**issue (bug_risk):** Dockerfiles that use ARG substitution in FROM lines may not be handled correctly.
This implementation only works when the FROM image is a literal value. In cases like `ARG BASE_IMAGE` followed by `FROM ${BASE_IMAGE}`, `parseFromImage` will return `${BASE_IMAGE}`, which will not be a valid reference for `parseImageRef`/`generateImageSBOM`. If these patterns should be supported, you’ll need to resolve ARGs (including defaults/env), or detect non-literal FROM targets and fail fast with a clear error.
</issue_to_address>
### Comment 2
<location path="src/providers/oci_dockerfile.js" line_range="44-52" />
<code_context>
+ if (/^FROM\s+/i.test(trimmed)) {
+ // Extract image ref: FROM [--platform=...] image [AS name]
+ const withoutFrom = trimmed.replace(/^FROM\s+/i, '')
+ // Skip optional --platform flag
+ const withoutFlags = withoutFrom.replace(/^--\S+\s+/, '')
+ // Take only the image part (before AS alias)
+ const parts = withoutFlags.split(/\s+/)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Flag stripping on FROM lines only removes a single leading flag, which may miss valid Dockerfile syntax.
The Dockerfile syntax allows multiple flags before the image (e.g. `FROM --platform=$BUILDPLATFORM --some-flag image AS name`), but the current regex only removes a single leading `--...` token. With additional flags, `parts[0]` could still be a flag instead of the image. Consider stripping all leading `--...` tokens (e.g. in a loop) or splitting and filtering out `--*` tokens before selecting the image reference.
```suggestion
if (/^FROM\s+/i.test(trimmed)) {
// Extract image ref: FROM [--platform=...] [--flags...] image [AS name]
const withoutFrom = trimmed.replace(/^FROM\s+/i, '')
// Split into tokens and drop all leading flag tokens (starting with "--")
const tokens = withoutFrom.split(/\s+/).filter(Boolean)
let imageIndex = 0
while (imageIndex < tokens.length && tokens[imageIndex].startsWith('--')) {
imageIndex++
}
if (imageIndex < tokens.length) {
// The first non-flag token is the image; ignore any following AS alias
lastFrom = tokens[imageIndex]
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #569 +/- ##
==========================================
- Coverage 90.75% 90.70% -0.05%
==========================================
Files 36 38 +2
Lines 7766 7921 +155
Branches 1353 1373 +20
==========================================
+ Hits 7048 7185 +137
- Misses 718 736 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review — Suggestion 1 (parseFromImage robustness): Classified as code change request (upgraded from suggestion) — covered by inline comment sub-tasks TC-4977 and TC-4978. Suggestion 2 (synchronous fs.readFileSync): Classified as suggestion — the project consistently uses |
Verification Report for TC-4937 (commit 905c336)
Overall: WARNTwo reviewer suggestions from sourcery-ai[bot] were upgraded to code change requests based on project conventions (CONVENTIONS.md §Error Handling):
Both are scoped improvements to the FROM line parser. All acceptance criteria pass and CI is green. This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0. |
The parseFromImage function previously only stripped a single --flag token using a regex. Dockerfile syntax allows multiple flags before the image reference. Replace the single regex with a loop that skips all leading --flag tokens. TC-4977 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Assisted-by: Claude Code
When a Dockerfile uses ARG substitution in FROM lines (e.g.
FROM ${BASE_IMAGE}), parseFromImage now throws a clear error
instead of passing the unresolved variable to downstream functions.
TC-4978
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Assisted-by: Claude Code
Verification Report for TC-4937 (commit dd053df)
Overall: PASSAll review feedback from the prior verification has been addressed:
Both sub-tasks are Done. No new issues found. This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0. |
a-oren
left a comment
There was a problem hiding this comment.
Verification Report — TC-4937
Verdicts
| Check | Verdict | Summary |
|---|---|---|
| Scope Containment | ✅ PASS | All 11 files map to the task spec or justified expansions from review feedback |
| Diff Size | ✅ PASS | ~291 additions / ~13 deletions proportionate for new provider + review-driven enhancements |
| Commit Traceability | Commit caade06 does not reference TC-4937 or any Jira ID |
|
| Sensitive Patterns | ✅ PASS | No hardcoded secrets, credentials, or API keys found |
| CI Status | ✅ PASS | All 5 checks pass (Node 22, Node 24, Sourcery, PR title, commit messages) |
| Acceptance Criteria | ✅ PASS | All 5 code-verifiable criteria satisfied |
| Verification Commands | ✅ PASS | npm test and npm run lint pass in CI |
| Test Quality | JSDoc comments on individual test cases diverge from project convention (cosmetic) | |
| Test Change Classification | ADDITIVE | New test file only; no existing tests modified or removed |
Findings
Commit Traceability — WARN
Commit caade06 ("feat(oci): normalize Docker Hub refs and use tree-sitter for Dockerfile parsing") contains significant changes (tree-sitter migration, Docker Hub normalization, env var rename, suffixed Dockerfile support) but does not reference TC-4937 or any Jira ticket in its body. The other 3 commits properly reference their tasks (TC-4937, TC-4977, TC-4978).
Test Quality — WARN
The new oci_dockerfile.test.js has /** */ JSDoc comments above each test() call. Existing test files (oci_images.test.js, golang_gomodules.test.js, python_pip.test.js) have no per-test JSDoc comments — only rust_cargo.test.js uses JSDoc, but on helper functions, not tests. This is cosmetic and not blocking.
Review Feedback
| Thread | Reviewer | Classification | Status |
|---|---|---|---|
| 3490346155 | sourcery-ai | Code change request (ARG substitution) | Addressed — TC-4978 (Closed) |
| 3490346161 | sourcery-ai | Code change request (flag stripping) | Addressed — TC-4977 (Closed) |
| review-body-4590175943 | sourcery-ai | Code change request + suggestion | Addressed in commits |
| 3490945019 | Strum355 | Code change request (suffixed Dockerfiles) | Addressed in caade06 |
| 3490971432 | Strum355 | Code change request (tree-sitter parser) | Addressed in caade06 |
All review feedback has been addressed. No outstanding code change requests.
This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0.
…le parsing Normalize Docker Hub image references in PURL generation so all FROM formats (node:22, docker.io/node:22, docker.io/library/node:22) produce the same repository_url=docker.io/node qualifier, matching the Hummingbird hardened image index. Migrate Dockerfile FROM parsing from regex to tree-sitter-containerfile for robust AST-based extraction. Rename TRUSTIFY_DA_RECOMMENDATIONS_ENABLED to TRUSTIFY_DA_RECOMMEND and use getCustom() for consistent env/option lookup. Support suffixed Dockerfile variants (e.g. Dockerfile.dev). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
oci_dockerfile.jsprovider that recognizesDockerfileandContainerfilemanifests for component and stack analysisgenerateImageSBOMto produce a CycloneDX SBOMisSupported,validateLockFile,readLicenseFromManifest,packageManagerName, and FROM line parsing--flagtokens from FROM lines, not just the first (TC-4977)Implements TC-4937
Test plan
isSupportedreturns true forDockerfileandContainerfile, false for othersvalidateLockFilealways returns truereadLicenseFromManifestreturns null🤖 Generated with Claude Code